Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow occam to cope with pack limitations #339

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented Oct 4, 2024

pack seems to be very limited in the options it will accept when building for multi-arch. It does not
seem to accept a path with a tgz as input and only works properly if you build in an exploded directory. Otherwise it appears to have built the cnb file but the structure of the bin directory is incorrect and the executables cannot be found.

Update the jam packager to work around this by exploding the tgz after creating it and then build in that
exploded directory.

It is still a good idea to build the tgz as that
filters the files based on what is specified in the buildpack.toml.

The unit tests pass, but more integration testing might be good. Once I have confirmation that people
are comfortable withe the PR I'll manually test on a few existing buildpacks in addition to the testing I've
already done on the npm-install buildpack modified for multi-arch -> https://github.com/mhdawson/npm-install/tree/test-multi (Note that this needs an updated jam that includes an updated packit with this change - paketo-buildpacks/packit#604)

Summary

Allow the ability to test buildpacks that have been updated to support multi-arch. Occam will still only test on the one architecture as the machine the tests are run on but with the update can run tests for buildpacks that support multi-arch, or buildpacks that don't.

Use Cases

Testing buildpacks that support multi-arch

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

pack seems to be very limited in the options it will
accept when building for multi-arch. It does not
seem to accept a path with a tgz as input and only
works properly if you build in an exploded directory.
Otherwise it appears to have built the cnb file but
the structure of the bin directory is incorrect and
the executables cannot be found.

Update the jam packager to work around this by exploding
the tgz after creating it and then build in that
exploded directory.

It is still a good idea to build the tgz as that
filters the files based on what is specified in the
buildpack.toml.

Signed-off-by: Michael Dawson <[email protected]>
@mhdawson mhdawson requested a review from a team as a code owner October 4, 2024 01:48
@mhdawson
Copy link
Member Author

mhdawson commented Oct 8, 2024

@ForestEckhardt any chance you could take a look, we need this to progress on getting Node.js buildpacks to support multi-arch.

@mhdawson
Copy link
Member Author

@robdimsdale any chance you could take a look?

@mhdawson
Copy link
Member Author

@ForestEckhardt, @robdimsdale any chance one of you two could take a look?

@ForestEckhardt
Copy link
Contributor

Should we address this at the source (i.e. jam) by either adding a different flag or something instead of trying to hack around it in occam?

@mhdawson
Copy link
Member Author

@ForestEckhardt the real fix is for pack to better support multi-arch so that all of the different options/use cases it supports for non-multiarch work but that is a much bigger effort that what is in this PR.

In this case I think jam just calls pack passing through some of the options provided by occam so it seems more confusing to me to try to add the work around in jam that says even through I called you with these pack options, convert the input and options to something else before calling pack.

@ForestEckhardt
Copy link
Contributor

@mhdawson So jam does not call pack internally at all it does however produce the compressed tar file that you are unwinding. We could add a flag to only produce a directory structure which is compatible with a multi-arch build as far as I understand. We could then set that flag in occam until we reconcile what artifacts we are going to release with our buildpacks going forward.

@mhdawson
Copy link
Member Author

I'm going to look at updating jam to see how that works out.

@mhdawson
Copy link
Member Author

After looking at changing jam insead for a bit, it looks like its more complicated that we thought. The tar creation is based of a list of files with access rights, timestamps and a directory structure. It would take more than just not zipping to create a copy that would ensure we get the same result.

@ForestEckhardt ForestEckhardt added the semver:minor A change requiring a minor version bump label Nov 19, 2024
@ForestEckhardt ForestEckhardt enabled auto-merge (rebase) November 19, 2024 18:37
@ForestEckhardt ForestEckhardt merged commit 9b665d9 into paketo-buildpacks:main Nov 19, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants